Skip to content

[CALCITE-6767] PERCENTILE_CONT/PERCENTILE_DISC function are not supported#5095

Open
xuzifu666 wants to merge 1 commit into
apache:mainfrom
xuzifu666:calcite-6767
Open

[CALCITE-6767] PERCENTILE_CONT/PERCENTILE_DISC function are not supported#5095
xuzifu666 wants to merge 1 commit into
apache:mainfrom
xuzifu666:calcite-6767

Conversation

@xuzifu666

Copy link
Copy Markdown
Member


# [CALCITE-4644] Add PERCENTILE_CONT and PERCENTILE_DISC aggregate functions.
# PERCENTILE_CONT / PERCENTILE_DISC without GROUP BY.
select

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result is validated in PostgreSQL in https://onecompiler.com/postgresql/44u4qdgpy and result is as expected

@xuzifu666 xuzifu666 force-pushed the calcite-6767 branch 2 times, most recently from 94660b4 to fb6f1ba Compare July 13, 2026 02:38
Comment thread core/src/test/resources/sql/agg.iq Outdated
!ok

# [CALCITE-6767] PERCENTILE_CONT/PERCENTILE_DISC syntax not supported.
# These sql was validated in PostgreSQL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sql -> sql programs
was -> were

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the remind, done.

final double rank = fraction * (n - 1);
final int lo = (int) Math.floor(rank);
final int hi = (int) Math.ceil(rank);
final double loValue = values.get(lo).doubleValue();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this won't work for DECIMAL values with large precisions, since double has only 53 bits of mantissa, which is insufficient to store large decimal values without a large precision loss. If the decimals are close in values, the error will be very large.

I think using BigDecimal is probably a better approach.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I’ve updated the handling logic to use BigDecimal instead of double.

@xuzifu666 xuzifu666 changed the title [CALCITE-6767] PERCENTILE_CONT/PERCENTILE_DISC syntax not supported [CALCITE-6767] PERCENTILE_CONT/PERCENTILE_DISC function are not supported Jul 14, 2026

!ok

# PERCENTILE_CONT / PERCENTILE_DISC with GROUP BY.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add tests for other numeric types, in particular DOUBLE, some large DECIMALS, some of which are very close to each other, and UNSIGNED?

@xuzifu666 xuzifu666 Jul 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had added related tests and validate in https://onecompiler.com/oracle/44v8xy4ft
Judging by the results:
-PostgreSQL, percentile_cont returns only double precision (53-bit mantissa, approximately 15–16 decimal digits); converting a 19-digit DECIMAL value to this type results in a loss of precision—for instance, 9999999999999999991 becomes 1e+19.
-Calcite (after modification): Uses BigDecimal throughout; the return type matches the ORDER BY column (i.e., DECIMAL(19,0)), fully preserving the 19-digit precision to yield the exact value 9999999999999999991.0.

The SQL standard may allow percentile_cont to return a double for exact numeric types, and PostgreSQL is one implementation that follows that (I also tested Oracle as well, and it behaves the same way). Of course, having Calcite preserve the input type's precision would be more user-friendly for those using DECIMAL—which is the approach currently adopted.

Few databases support the UNSIGNED type, so I chose DuckDB for testing; the results returned matched the expected test outcomes.(can be validated in https://shell.duckdb.org/):
using:

  select
    percentile_cont(0.5) within group (order by v) as c,
    percentile_disc(0.5) within group (order by v) as d
  from (values (cast(10 as uinteger)),
               (cast(20 as uinteger))) as t(v);

result is :

┌────────┬────────┐
│   c    │   d    │
│ double │ uint32 │
├────────┼────────┤
│ 15.0   │ 10     │
└────────┴────────┘

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to run tests on duckdb, these tests would produce the same results using integers. The only thing that is different is the overflow behavior.

@sonarqubecloud

Copy link
Copy Markdown

@xuzifu666 xuzifu666 added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants